5223 error message on nonvisible item request#5405
5223 error message on nonvisible item request#5405janeewheatley merged 14 commits intorubyforgood:mainfrom
Conversation
| end | ||
|
|
||
| def validate_visible_items!(child_ids) | ||
| invisible_item_requests = current_partner |
There was a problem hiding this comment.
You can reuse most if not all of the query that was created earlier. You can pass the query object into this method and just add the changes you need onto it.
There was a problem hiding this comment.
Having said that, all of this is starting to seem very heavy for controller logic. I think we should move most of this to the service class.
There was a problem hiding this comment.
This has been moved to the service class.
| family_requests_attributes = build_family_requests_attributes(params) | ||
| begin | ||
| family_requests_attributes = build_family_requests_attributes(params) | ||
| rescue ArgumentError => e |
There was a problem hiding this comment.
ArgumentError is a built-in Ruby exception that shouldn't be used for business logic validation. Probably best to create your own error class.
There was a problem hiding this comment.
Makes sense. Where would be the best place for the error class?
The file that looks most like is app/events/inventory_error.rb, so something like app/events/item_visibility_error.rb?
There was a problem hiding this comment.
If we're moving this to the service class, you can define the error inside the class itself.
There was a problem hiding this comment.
After looking at the service class, I felt it made more sense to just use the error handling that was already present. If you'd like me to create a custom error class instead just let me know
…nto 5223_error_message_on_nonvisible_item_request
|
Functional Testing
@Budmin Are you able to take a look at this? As an aside, I will create another ticket that addresses the issue where when an item is invisible, we are still seeing commas in the Item Needed |
|
@janeewheatley I'll take a look at this and get back to you shortly |
janeewheatley
left a comment
janeewheatley
left a comment
There was a problem hiding this comment.
See comments above
|
@janeewheatley I've pushed updates that makes the error message content more inline with what was expected. |
|
The error message now looks correct. Multiple items and children: |
|
@Budmin: Your PR |
4 participants
Resolves #5223
Description
The error message related to partners requesting
Itemswherevisible_to_partnersisfalsenow specifies which item and which children are causing the error.NOTE: There is a small change unrelated to the specific issue. In the
app/views/layouts/partners/application.html.erbfile, there were two overlapping "X" icons to dismiss the error message. I've removed one of them so now there's just one icon.Type of change
How Has This Been Tested?
I've added a test to the
spec/requests/partners/family_requests_requests_spec.rbfile that expects the format of the error message to include details about a non-visible item, and the child requesting it.Screenshots
Before:
After: